Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[201911] Add lgtm.yml #1901

Merged
merged 5 commits into from
Oct 11, 2021
Merged

[201911] Add lgtm.yml #1901

merged 5 commits into from
Oct 11, 2021

Conversation

stephenxs
Copy link
Collaborator

What I did
Add LGTM.yml for 201911

Signed-off-by: Stephen Sun stephens@nvidia.com

Why I did it

How I verified it

Details if related

@liat-grozovik
Copy link
Collaborator

@xumia do you have additional suggestion how to overcome the lgtm issues in 201911?

@liat-grozovik liat-grozovik mentioned this pull request Sep 12, 2021
Closed
@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs stephenxs force-pushed the fix-lgtm branch 2 times, most recently from ab2fc0a to c319d56 Compare September 18, 2021 14:10
@stephenxs
Copy link
Collaborator Author

To enable LGTM on 201911 branch, we need to cherry-pick the following PRs to 201911

@stephenxs stephenxs marked this pull request as ready for review September 18, 2021 14:22
@qiluo-msft
Copy link
Contributor

@abdosi Could you help cherry-pick above 2 PRs to 201911?

@abdosi
Copy link
Contributor

abdosi commented Oct 6, 2021

@stepanblyschak Above cherr-pick are done. Can you check why it is still failing ?

@abdosi
Copy link
Contributor

abdosi commented Oct 6, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Collaborator Author

Hi @abdosi
Thanks for cherry-picking them.
From the log I saw this

HEAD is now at 5866b49 lgtm: Merge of c319d569842ca2cf431621e7fe724ad2bffc4a8a into cccb59efa031b817c1936a74db969debbb9b2775

The commit cccb59ef is still the one before cherry-picking. is it because the submodule isn't updated?

@abdosi
Copy link
Contributor

abdosi commented Oct 8, 2021

check now. I have update submodule.

@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Add lgtm.yml
7bf8d05 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Copy link
Collaborator Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephenxs
Add some missing libs
fe46805 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Add missing libs
b8010ff 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs
Copy link
Collaborator Author

Compiling portsorch.cpp failed due to the following error:

void PortsOrch::doTask()
{
    constexpr auto tableOrder = {
        APP_PORT_TABLE_NAME,
        APP_LAG_TABLE_NAME,
        APP_LAG_MEMBER_TABLE_NAME,
        APP_VLAN_TABLE_NAME,
        APP_VLAN_MEMBER_TABLE_NAME,
    };
    ^error: ‘const std::initializer_list<const char*>{((const char* const*)(&<anonymous>)), 5}’ is not a constant expression
    for (auto tableName: tableOrder)
    {
        auto consumer = getExecutor(tableName);
        consumer->drain();
    }

Looks like some flags are not correct.

[2021-10-08 15:13:47] [build-stdout] g++ -DHAVE_CONFIG_H -I. -I.. -I .. -I ../warmrestart -I flex_counter -I debug_counter -g -DNDEBUG  -std=c++14 -Wall -fPIC -Wno-write-strings -I/usr/include/libnl3 -I/usr/include/swss -Werror -Wno-reorder -Wcast-align -Wcast-qual -Wconversion -Wdisabled-optimization -Wextra -Wfloat-equal -Wformat=2 -Wformat-nonliteral -Wformat-security -Wformat-y2k -Wimport -Winit-self -Winvalid-pch -Wlong-long -Wmissing-field-initializers -Wmissing-format-attribute -Wno-aggregate-return -Wno-padded -Wno-switch-enum -Wno-unused-parameter -Wpacked -Wpointer-arith -Wredundant-decls -Wstack-protector -Wstrict-aliasing=3 -Wswitch -Wswitch-default -Wunreachable-code -Wunused -Wvariadic-macros -Wno-switch-default -Wno-long-long -Wno-redundant-decls -I /usr/include/sai -I/opt/work/lgtm-workspace/usr/include -I/opt/work/lgtm-workspace/usr/include/swss -I/opt/work/lgtm-workspace/usr/include/sai  -g -O2 -MT orchagent-portsorch.o -MD -MP -MF .deps/orchagent-portsorch.Tpo -c -o orchagent-portsorch.o `test -f 'portsorch.cpp' || echo './'`portsorch.cpp
[2021-10-08 15:13:48] [build-stderr] portsorch.cpp: In member function ‘virtual void PortsOrch::doTask()’:
[2021-10-08 15:13:48] [build-stderr] portsorch.cpp:2727:5: error: ‘const std::initializer_list<const char*>{((const char* const*)(&<anonymous>)), 5}’ is not a constant expression
[2021-10-08 15:13:48] [build-stderr]  2727 |     };
[2021-10-08 15:13:48] [build-stderr]       |     ^
[2021-10-08 15:13:56] [build-stderr] make[2]: *** [Makefile:706: orchagent-portsorch.o] Error 1
[2021-10-08 15:13:56] [build-stdout] make[2]: Leaving directory '/opt/src/orchagent'
[2021-10-08 15:13:56] [build-stdout] make[1]: Leaving directory '/opt/src'
[2021-10-08 15:13:56] [build-stderr] make[1]: *** [Makefile:404: all-recursive] Error 1
[2021-10-08 15:13:56] [build-stderr] make: *** [Makefile:336: all] Error 2

@stephenxs
Copy link
Collaborator Author

@prsunny can you help to check why this error occurred during LGTM compiling? The error didn't occur if I build it in slave docker.

Fix LGTM error in portsorch.cpp
fa2c96e 
Signed-off-by: stephens <stephens@contoso.com>
@stephenxs
Copy link
Collaborator Author

@prsunny can you help to check why this error occurred during LGTM compiling? The error didn't occur if I build it in slave docker.

This error has been fixed by #1250. We need to cherry-pick it as well.

@xumia
Copy link
Collaborator

xumia commented Oct 11, 2021 •
edited
Loading

We can still see the LGTM failure in the PR, it will be resolved after the PR merged, because of LGTM failure on branch 201911.

https://lgtm.com/projects/g/Azure/sonic-swss/logs/rev/pr-99c1bbe81ccb8c054a9da6d2eb9795bf89750d08/lang:cpp/stage:Build%20201911_901e2efb4f5c6a1179068b43e6a837dfff19d9d9

Commit fa2c96e is the latest commit.
Commit ac179db is the commit merged to branch 201911, works as expected.
Commit 901e2ef is the latest commit on branch 201911, it does not work, it will work after merging the lgtm configuration.

@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Oct 11, 2021 via email

@stephenxs
Revert "Fix LGTM error in portsorch.cpp"
014c6e0 
To fix LGTM in this PR is for dry run only.
It should be done by cherry-picking PR#1250
With portsorch.cpp fixed, dry run passed.

This reverts commit fa2c96e.
@qiluo-msft qiluo-msft merged commit acdb033 into sonic-net:201911 Oct 11, 2021
@stephenxs stephenxs deleted the fix-lgtm branch October 11, 2021 23:03
@stephenxs stephenxs mentioned this pull request Oct 14, 2021
Closed
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
@renukamanavalan
generic_updater: Fix service validator related issues (sonic-net#1901)
095bf54 
1) Copy generic_updater_config.conf.json as part of install
2) Read conf file from install dir
3) Drop empty keys & tables upon jsonpatch.JsonPatch.apply to be in sync with
   redis update
4) Prefix service_validator module path with "generic_updater"
@ayurkiv-nvda ayurkiv-nvda mentioned this pull request Jun 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xumia xumia xumia approved these changes

@qiluo-msft qiluo-msft qiluo-msft approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@stephenxs @liat-grozovik @mssonicbld @qiluo-msft @abdosi @xumia